gh-133390: Support basic completion for sqlite3 command-line interface #133393
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
I think we need a separate nodule for this because if we want to have a smarter completion later the code is better to be isolated, so something like sqlite3._completer. If possible, can we have an autogenerated list of keywords? and those keywords could also be stored in a (private) module-level list. |
Misc/NEWS.d/next/Library/2025-05-05-03-14-08.gh-issue-133390.AuTggn.rst
Outdated
Show resolved
Hide resolved
|
Please add tests |
|
Yes, tests is coming. I will try to find a way to autogenerate list of keywords. |
|
You can mark the pr as a draft. |
tanloong
force-pushed
the
sqlite3-cli-completion
branch
8 times, most recently
from
782a599 to
5005d85
Compare
|
Added tests and moved keyword list to module level. Keywords seems not easy to auto-generate, the SQLite document says they can be accessed by |
There was a problem hiding this comment.
With #133447 in mind the keywords list would be quite handy (I think hardcoding is fine, it doesn't change often anyway). Maybe it could be moved elsewhere, what do you think Benedikt?
tanloong
force-pushed
the
sqlite3-cli-completion
branch
from
15019ea to
22652ae
Compare
|
Hi @picnixz, as Stan mentioned, beta freeze is around the corner, but I really hope this can be shipped in 3.14, do you think there is a chance to merge it before beta freeze? |
There was a problem hiding this comment.
On my side it looks great but I'll also want @erlend-aasland to have a look as he's the main maintainer. Maybe he'l ask for some stylistic changes (in which case I'm sorry if I suggested something wrong).
|
|
||
| @requires_subprocess() | ||
| class CompletionTest(unittest.TestCase): | ||
| PS1 = "sqlite> " |
There was a problem hiding this comment.
This is no longer freely customizable by users via sys.ps1 ?
Thank you so much for your review. Really appreciate your guidance! |
|
The Unfortunately I didn't find a Can we remove this |
|
Let's just check that the completion of nothing returns all keywords. You can sort the |
GNU Readline always sorts completions before displaying them. There is a [rl_sort_completion_matches](https://git.savannah.gnu.org/cgit/readline.git/tree/complete.c#n411) in Readline's source code but it's not exposed as a config flag.
Modules/_sqlite/module.c
Outdated
| goto error; | ||
| } | ||
| } | ||
| if (PyModule_Add(module, "SQLITE_KEYWORDS", keywords) < 0) { |
There was a problem hiding this comment.
PyModule_Add() "eats" keywords, so Py_XDECREF should not be called after error. Just return -1. Or even return the result of PyModule_Add().
There was a problem hiding this comment.
cc @encukou who implemented this I believe.
There was a problem hiding this comment.
May I try to update the C code. As this PR is sent from my fork, it would be more convenient that I make the change. I will convert if I mess it up.
encukou
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit 8d4f659 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133393%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
There are failures on FreeBSD, and on Ubuntu and some Fedora/RHEL configurations :( |
|
Yes, I will look into it. |
This adds tab-completion for the 147 SQLite Keywords. A whitespace is appended to completion candidates for users' convenience and to mimic the behavior of sqlite3 tool.